Skip to content

Do not create a new node instance for the transform listener#5912

Closed
Timple wants to merge 1 commit intoros-navigation:mainfrom
nobleo:fix/transform-nodes
Closed

Do not create a new node instance for the transform listener#5912
Timple wants to merge 1 commit intoros-navigation:mainfrom
nobleo:fix/transform-nodes

Conversation

@Timple
Copy link
Copy Markdown
Contributor

@Timple Timple commented Jan 28, 2026 

Before:
    $ ros2 node list
    /controller_server
    /transform_listener_impl_650b885185e8

After:
    $ ros2 node list
    /controller_server

For Maintainers:

  • Check that any new parameters added are updated in docs.nav2.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists
  • Should this be backported to current distributions? If so, tag with backport-*.

@Timple
Do not create a new node instance for the transform listener
30cee08 
    Before:
        $ ros2 node list
        /controller_server
        /transform_listener_impl_650b885185e8

    After:
        $ ros2 node list
        /controller_server

Signed-off-by: Tim Clephas <tim.clephas@nobleo.nl>
@mini-1235
Copy link
Copy Markdown
Collaborator

mini-1235 commented Jan 28, 2026

Modifying in costmap_ros will likely cause an issue, see #5406

@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Jan 28, 2026

We may even see that in action in CI. All of the system tests failed. I'm going to retrigger just to make sure it wasn't a fluke though.

@Timple
Copy link
Copy Markdown
Contributor Author

Timple commented Jan 29, 2026

Interesting...

As mitigation we can also give it a move clarifying name, like controller_server_tf by mimicing the implementation here: https://github.com/ros2/geometry2/blob/rolling/tf2_ros/src/transform_listener.cpp

@mini-1235
Copy link
Copy Markdown
Collaborator

mini-1235 commented Jan 29, 2026

I think Costmap2DROS::on_configure(const rclcpp_lifecycle::State & /*state*/) is called not only by the controller server but also by the planner server. If we rename it to something like costmap_ros_tf, that would result in duplicate node names, which I don't think better than the current situation (?)

@Timple
Copy link
Copy Markdown
Contributor Author

Timple commented Jan 29, 2026

Well in that case let's forget about it I guess..
It was merely a cosmetic thing for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Timple @mini-1235 @SteveMacenski